Skip to content

Conversation

@626-ju
Copy link
Collaborator

@626-ju 626-ju commented May 23, 2025

https://panda-sprint5.netlify.app/

요구사항

기본

  • 중고마켓 페이지 주소는 “/items” 입니다.

  • 페이지 주소가 “/items” 일때 상단네비게이션바의 '중고마켓' 버튼의 색상은 “3692FF”입니다.

  • 상단 네비게이션 바는 이전 미션에서 구현한 랜딩 페이지와 동일한 스타일로 만들어 주세요.

  • 상품 데이터 정보는 https://panda-market-api.vercel.app/docs/#/ 에 명세된 GET 메소드 “/products” 를 사용해주세요.

  • '상품 등록하기' 버튼을 누르면 “/additem” 로 이동합니다. ( 빈 페이지 )

  • 전체 상품에서 드롭 다운으로 “최신 순” 또는 “좋아요 순”을 선택해서 정렬을 할 수 있습니다.

  • 중고마켓 반응형
    베스트 상품
    Desktop : 4개 보이기
    Tablet : 2개 보이기
    Mobile : 1개 보이기
    전체 상품
    Desktop : 10개 보이기
    Tablet : 6개 보이기
    Mobile : 4개 보이기

심화

  • 페이지 네이션 기능을 구현합니다.

주요 변경사항

스크린샷

header

originalgnb

logingnb

itemgnb

PC

localhost_5173_items (4)

태블릿

localhost_5173_items (1) 1

모바일

localhost_5173_items (3) 1

정렬

대체이미지추가

페이지네이션

page

멘토에게

  • 최대한 보기 좋게 다듬고 싶었는데 프롭으로 내려주는 게 많아지니까 너무 보기 힘들게 된 것 같아요.
    로직도 최대한 안 꼬이게 하고 싶었는데 서로 영향 주는 걸 풀려고 해도 생각한 만큼 잘 되지 않았습니다.
    보기 힘드실 것 같아서 죄송해요 ㅠㅠㅠㅠㅠ

  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@626-ju 626-ju added the 순한맛🐑 마음이 많이 여립니다.. label May 23, 2025
@626-ju 626-ju changed the title React 김성주 sprint5 김성주 [sprint5] May 23, 2025
@626-ju 626-ju changed the title 김성주 [sprint5] [김성주] sprint5 May 23, 2025
@626-ju 626-ju requested a review from addiescode-sj May 23, 2025 07:12
Copy link
Collaborator

@addiescode-sj addiescode-sj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다!
첫 리액트 미션이다보니 변경사항이 너무 많아서 리뷰하는데 조금 힘들었지만, 열심히 해주셔서 보람있었어요 :) useValidate 훅 구조 관련해서 코멘트 드린거 먼저 봐주시고, 전체적으로 코드 스타일과 재사용성을 고려해 개선하는 방법 위주로 피드백 드렸으니 참고해서 리팩토링 진행해보시면 좋을것같아요 :)

주요 리뷰 포인트

  • 불필요한 리렌더링 방지 및 코드 복잡도 개선 (useValidate 훅)
  • 코드 스타일 관련 피드백
  • 유지보수를 고려한 개발
  • 디렉토리 구조

src/App.jsx Outdated
return (
<BrowserRouter>
<Routes>
<Route element={<Header></Header>}>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<Route element={<Header></Header>}>
<Route element={<Header />}>

Self closing으로 쓰셔도 됩니다!

import HeaderAuth from "./HeaderAuth";

function Header() {
const [isLogined, setIsLogined] = useState(sessionStorage.getItem("logined"));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const [isLogined, setIsLogined] = useState(sessionStorage.getItem("logined"));
const [isLoggedIn, setIsLoggedIn] = useState(sessionStorage.getItem("loggedIn"));

문법상 isLoggedIn이 맞습니다.
간단한 단어 교정이나 오타 검증은 아래 extension 도움을 받아보세요! :)

streetsidesoftware.code-spell-checker

@@ -0,0 +1,30 @@
import { createContext, useState } from "react";

export const ProductAllContext = createContext();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

네이밍이 조금 모호하네요. api 요청에 의한 response data라면 ProductData정도가 적당할것같습니다 :)

Copy link
Collaborator

@addiescode-sj addiescode-sj May 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 context는 src 바로 밑에 있어서, 일반적으로 단일 페이지가 아닌 여러 페이지에서 공유되는 공용 목적으로 쓰입니다.
만약 특정 페이지에서만 쓰이는 context라면 사용하는 입장에서 찾기 쉽게 폴더 및 파일을 이동하시는게 좋을것같아요.

아래 아티클 참고해보시고, 성주님이 쓰기 좋은 방식으로 개선해보세요!
참고

Comment on lines 27 to 32
useEffect(() => {
if (innerWidth === null) return;

const newPageSize = reCalculatePageSize(type, innerWidth)
setPageSize(newPageSize)
}, [innerWidth]);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type이 의존성 배열에 포함되어있지않아 type이 변경되어도 pageSize가 재계산되지 않을 수 있다는 문제가 생길 수 있어요. 아마 eslint 사용하시면 기본 규칙에 포함되어있어서 관련해서 워닝이 표시될텐데 사용해보시는걸 추천드립니다! :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

구조상, 이 커스텀훅을 사용하는 모든 컴포넌트에서 입력 필드가 변경될때마다 상태가 바뀌고, 해당 훅을 사용하는 모든 컴포넌트가 리렌더링되는 문제가 발생해요.

이 훅을 유지하면서도(구조를 크게 바꾸지않으면서도) 문제를 개선하기 위해서는, 각 입력 필드의 유효성 검사 결과를 선택적으로(필요할때만) 구독할 수 있도록 구현해주는게 좋을것같아요.

useReducer 훅을 사용해 개별적인 상태 업데이트 로직을 관리하고, 항상 모든 입력필드의 state 변경을 감지하지않고 특정 state가 바뀌지 않는 한 같은 결과를 재사용할수있도록 메모이제이션 해주면 어떨까요?

예시를 작성해드릴게요 :)

  • useReducer를 사용해 상태 업데이트
const initialState = {
  values: {},
  errors: {},
  errorMessages: {},
  isValid: {},
};

function validationReducer(state, action) {
  switch (action.type) {
    case "SET_VALUE": {
      const { name, value } = action.payload;
      const validator = validRuleObj[name];
      const isValid = validator.isValid(value, state.values["user-password"]);
      const errorMessage = isValid ? "" : validator.getErrorMessage(value);

      return {
        ...state,
        values: { ...state.values, [name]: value },
        errors: { ...state.errors, [name]: !isValid },
        errorMessages: { ...state.errorMessages, [name]: errorMessage },
        isValid: { ...state.isValid, [name]: isValid },
      };
    }
    default:
      return state;
  }
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그 다음 useValidate 훅에서 해당 reducer를 사용해 상태 업데이트를 관리하면서도, 특정 state가 바뀌지 않는 한 같은 결과를 재사용할수있도록 메모이제이션해주면:

export function useValidate() {
  const [state, dispatch] = useReducer(validationReducer, initialState);

  const setValue = useCallback((name, value) => {
    dispatch({ type: "SET_VALUE", payload: { name, value } });
  }, []);

  const getFieldState = useCallback(
    (name) => {
      return {
        value: state.values[name] || "",
        error: state.errors[name] || false,
        errorMessage: state.errorMessages[name] || "",
        isValid: state.isValid[name] || false,
      };
    },
    [state]
  );

  return {
    setValue,
    getFieldState,
  };
}

이런 장점이 생겨요.

  • useReducer를 사용하여 상태 업데이트 로직을 더 예측 가능하게 (간소화) 만들 수 있습니다.
  • 각 입력 필드의 상태를 선택적으로 구독할 수 있어 불필요한 리렌더링을 방지합니다.
  • 사용하는 입장에서도, getFieldState 함수를 통해 각 필드의 상태를 쉽게 가져올 수 있습니다.
  • 컴포넌트에서 필드별 유효성 검사를 수행할때마다 훅 인스턴스를 생성하지않아도되고 (복잡도를 떨어트리는것또한 준수한 코드 퀄리티를 유지하기위해 중요), 하나의 훅 실행으로 모든 상태를 관리하게 만들 수 있습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이렇게 구조를 바꿔주면 유효성 검사를 수행해야하는 컴포넌트에서는

const validation = useValidate();
const emailState = validation.getFieldState("user-email");

// 입력 필드에서
<input
  value={emailState.value}
  onChange={(e) => validation.setValue("user-email", e.target.value)}
/>

이런식으로 useValidate 훅을 한번만 실행하고, 상태 업데이트를 원하는 입력 필드만 선택적으로 연결해 사용하는게 가능해지죠? :)

Comment on lines 11 to 13
const toItemsNavigation = useNavigate();
const emailInput = useValidate();
const passwordInput = useValidate();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위에 useValidate 훅 구조 개선 관련해 코멘트 참고해보시고, 이런식으로 여러개의 훅 인스턴스를 생성하지않도록 해봅시다! :)

Comment on lines 10 to 13
<ProductsFavorite></ProductsFavorite>
<ProductAllContextProvider>
<ProductsAll></ProductsAll>
</ProductAllContextProvider>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<ProductsFavorite></ProductsFavorite>
<ProductAllContextProvider>
<ProductsAll></ProductsAll>
</ProductAllContextProvider>
<ProductsFavorite />
<ProductAllContextProvider>
<ProductsAll />
</ProductAllContextProvider>

<section className={styles.items__all}>
<div className={styles[`items__all-filter`]}>
<h2>전체 상품</h2>
<ProductsFilterBar></ProductsFilterBar>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요렇게 많이 쓰시네요 내부에 children이 없으면 <ProductsFilterBar /> 이렇게 쓰시는게 간편합니다!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

굳굳! 재사용 가능한 로직(페이지네이션 기능과 관련된 로직)은 커스텀훅으로 따로 또 분리해주면 재사용성을 극대화할수있어요 :)

나중에 새로운 형태의 Pagination UI가 추가될때 이 Pagination 컴포넌트에 props로 추가해 조건부 렌더링을 하기보다는 새 Pagination 컴포넌트를 만들어서 커스텀 훅을 사용해주면 용이하겠죠?

혹은, 기존 Pagination 컴포넌트에 합성 패턴을 사용해 UI도 재사용 가능한 단위로 분리하고 조합해 사용해주는 방법도 있고요 :)

참고

626-ju added 3 commits May 26, 2025 17:53
-useReduce -> 로직 간소화, 불필요한 인스턴스생성 제거
-useCallback, React.memo -> 불필요한 인풋 리렌더링 방지
-Header -> logined x loggedIn으로 이름 수정
-useResizeInnerWidht 훅 내부 useEffect deps에 type 추가
-ProductsAllContextProvider -> ProductDataProvider로 이름 변경
-context폴더 삭제 -> ProductDataProvider폴더 pages/Items폴더 내부에 위치
@addiescode-sj addiescode-sj merged commit 4c79977 into codeit-bootcamp-frontend:React-김성주 May 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

순한맛🐑 마음이 많이 여립니다..

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants